[GIE] Support `PathExpand` with `OPT=ALL_V_E` in GIE #2841

BingqingLyu · 2023-06-08T11:30:33Z

What do these changes do?

As titled.

You can query a path expand with both vertices and edges preserved by specify the ResultOpt as ALL_V_E. e.g.,

Related issue number

Fixes #2776

…nd edges

…h with AllVE

codecov-commenter · 2023-06-08T12:42:56Z

Codecov Report

Merging #2841 (0362a94) into main (bd89520) will decrease coverage by 2.72%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2841      +/-   ##
==========================================
- Coverage   45.30%   42.59%   -2.72%     
==========================================
  Files          99       99              
  Lines       10668    10657      -11     
==========================================
- Hits         4833     4539     -294     
- Misses       5835     6118     +283

see 16 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 086fa63...0362a94. Read the comment docs.

longbinlai · 2023-06-12T02:39:10Z

interactive_engine/compiler/src/main/java/com/alibaba/graphscope/common/jna/type/ResultOpt.java

@@ -20,7 +20,8 @@

 public enum ResultOpt implements IntEnum<ResultOpt> {
    EndV,
-    AllV;
+    AllV,
+    AllVe;


AllVe is strange, either AllVE, if the compiler complains, change everything to full name, ie., EndVertex, AllVertex, AllVertexEdge

Updated as AllVE.

longbinlai · 2023-06-12T02:40:15Z

interactive_engine/executor/ir/core/src/plan/ffi.rs

@@ -2175,6 +2175,7 @@ mod graph {
    pub enum PathResultOpt {
        EndV = 0,
        AllV = 1,
+        AllVe = 2,


Same as the above AllVe

Updated as AllVE.

longbinlai · 2023-06-12T02:56:37Z

interactive_engine/executor/ir/graph_proxy/src/apis/graph/element/path.rs

-    SimpleAllV(Vec<Vertex>),
-    EndV((Vertex, usize)),
-    SimpleEndV((Vertex, Vec<ID>)),
+    AllV(Vec<VertexOrEdge>),


If this is already a complex combination, we may want to include two enum, one is PathOpt, one is ResultOpt, as what we have defined in proto. namely,

pub struct GraphPath { path_opt: PathOpt, reult_opt: ResultOpt, data: Vec<VertexOrEdge> }

Some path may not need to preserve the whole path data of Vec<VertexOrEdge> (e.g., when EndV or SimpleEndV here).

longbinlai · 2023-06-12T03:29:59Z

interactive_engine/executor/ir/core/src/plan/physical.rs

@@ -352,6 +353,9 @@ fn build_and_try_fuse_get_v(builder: &mut JobBuilder, mut get_v: pb::GetV) -> Ir
                    return Ok(());
                }
            }
+        } else if let physical_pb::physical_opr::operator::OpKind::Path(ref _path) = op_kind {
+            // make opt of getV after path expand as End.
+            get_v.opt = physical_pb::get_v::VOpt::End as i32;


using std::mem::transmute ?

longbinlai · 2023-06-12T03:32:44Z

interactive_engine/executor/ir/graph_proxy/src/apis/graph/mod.rs

@@ -168,7 +168,7 @@ impl QueryParams {
    }

    pub fn is_queryable(&self) -> bool {
-        !(self.labels.is_empty() && self.filter.is_none() && self.limit.is_none() && self.columns.is_none())
+        !(self.filter.is_none() && self.limit.is_none() && self.columns.is_none())


Why remove labels.is_empty() here?

I removed this function as it is not used.

longbinlai · 2023-06-12T03:33:05Z

interactive_engine/executor/ir/runtime/src/assembly.rs

                    if let Some(edge_expand) = base.edge_expand.take() {
-                        base_expand_plan.push(edge_expand.into());
+                        if pb::path_expand::ResultOpt::AllVe as i32 == path.result_opt {


use std::mem::transmute

longbinlai · 2023-06-12T03:35:11Z

interactive_engine/executor/ir/runtime/src/process/operator/map/get_v.rs

+            // no label constraint
+            Ok(true)
+        } else {
+            if let Some(label) = label {


Why we want to allow None label input in the function on the one hand, while throwing error while calling None label on the other hand? Simple pass &LabelId instead of Option<&LabelId>, no?

This function is to test if a vertex satisfied the label condition (i.e., filter by label):

if without label condition, just return true (ignoring whether the label of RuntimeVertex is Some(&LabelId) or None);

if with label condition, then it would further verify the RuntimeVertex label. And actually, RuntimeVertex will always carry the label for now (i.e., won't throw the error actually).

I know, then this function should not accept Option<&LabelId>, but let the called judge.

longbinlai · 2023-06-12T03:38:31Z

interactive_engine/executor/ir/integrated/tests/pathxd_test.rs

+        let mut result_collection: Vec<Vec<ID>> = vec![];
+        let mut expected_result_paths = vec![
+            // v1, e[1->2], v2
+            vec![1, 1, 2, 2],


I think making the element String, and if it is a vertex, turn ID into String, and if it is an Edge, turn (ID, ID) into string. This will make the results more clear.

interactive_engine/executor/ir/graph_proxy/src/apis/graph/element/path.rs

interactive_engine/executor/ir/integrated/tests/pathxd_test.rs

longbinlai · 2023-06-12T11:19:28Z

interactive_engine/executor/ir/graph_proxy/src/apis/graph/element/path.rs

 impl GraphPath {
-    pub fn new(entry: Vertex, path_opt: PathOpt, result_opt: ResultOpt) -> Self {
+    pub fn new<E: Into<VertexOrEdge>>(entry: E, path_opt: PathOpt, result_opt: ResultOpt) -> Self {
        match result_opt {
            ResultOpt::EndV => match path_opt {


This ResultOpt and PathOpt are both from pb, includes the whole path, like: pb::xx::xx::ResultOpt. In addition, I know ResultOpt::AllVe is generated, but can we avoid generating this name (that is inconsistent with our naming convention).

BingqingLyu added 4 commits June 8, 2023 13:32

[Proto] add AllVE opt for PathExpand to preserve path with vertices a…

decfe03

…nd edges

[Physical Plan] minor refine in the physical plan to support GraphPat…

324caa3

…h with AllVE

[GraphPath] support GraphPath with vertices and edges

9309a90

[CI TEST] add ci tests for PathExpand

34bab4e

BingqingLyu marked this pull request as draft June 8, 2023 11:30

minor fix

a1fc906

BingqingLyu marked this pull request as ready for review June 9, 2023 01:58

BingqingLyu requested a review from longbinlai June 9, 2023 01:59

[Compiler] support ResultOpt=AllVe in Compiler

99f5ce4

BingqingLyu changed the title ~~[GIE/Runtime] Support PathExpand with OPT=AllVE in Runtime~~ [GIE] Support PathExpand with OPT=ALL_VE in GIE Jun 9, 2023

format

85b5f52

BingqingLyu requested a review from lidongze0629 June 9, 2023 02:25

Merge branch 'main' into path_expand_e

8a89335

longbinlai approved these changes Jun 12, 2023

View reviewed changes

longbinlai self-requested a review June 12, 2023 03:03

longbinlai reviewed Jun 12, 2023

View reviewed changes

shirly121 and others added 2 commits June 12, 2023 13:15

[GIE Compiler] support ResultOpt=AllVe in Compiler

956937c

minor fix

260887c

BingqingLyu changed the title ~~[GIE] Support PathExpand with OPT=ALL_VE in GIE~~ [GIE] Support PathExpand with OPT=ALL_V_E in GIE Jun 12, 2023

BingqingLyu and others added 2 commits June 12, 2023 15:58

minor fix

5441679

Merge branch 'main' into path_expand_e

2d7dcb9

longbinlai reviewed Jun 12, 2023

View reviewed changes

interactive_engine/executor/ir/graph_proxy/src/apis/graph/element/path.rs Show resolved Hide resolved

longbinlai reviewed Jun 12, 2023

View reviewed changes

interactive_engine/executor/ir/integrated/tests/pathxd_test.rs Outdated Show resolved Hide resolved

BingqingLyu and others added 3 commits June 12, 2023 17:53

minor fix

a755903

minor fix according to comment

3b0e022

Merge branch 'main' into path_expand_e

9ec9174

longbinlai reviewed Jun 12, 2023

View reviewed changes

BingqingLyu and others added 2 commits June 13, 2023 10:55

minor fix

c9e13e6

Merge branch 'main' into path_expand_e

0362a94

Merge branch 'main' into path_expand_e

9c8244a

BingqingLyu merged commit e9af91a into alibaba:main Jun 13, 2023

BingqingLyu deleted the path_expand_e branch June 13, 2023 06:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GIE] Support `PathExpand` with `OPT=ALL_V_E` in GIE #2841

[GIE] Support `PathExpand` with `OPT=ALL_V_E` in GIE #2841

BingqingLyu commented Jun 8, 2023 •

edited

Loading

codecov-commenter commented Jun 8, 2023 •

edited

Loading

longbinlai Jun 12, 2023

BingqingLyu Jun 12, 2023

longbinlai Jun 12, 2023

BingqingLyu Jun 12, 2023

longbinlai Jun 12, 2023

BingqingLyu Jun 12, 2023

longbinlai Jun 12, 2023

BingqingLyu Jun 12, 2023

longbinlai Jun 12, 2023

BingqingLyu Jun 12, 2023

longbinlai Jun 12, 2023

BingqingLyu Jun 12, 2023

longbinlai Jun 12, 2023

BingqingLyu Jun 12, 2023 •

edited

Loading

longbinlai Jun 12, 2023

longbinlai Jun 12, 2023

BingqingLyu Jun 12, 2023

longbinlai Jun 12, 2023

BingqingLyu Jun 13, 2023

[GIE] Support PathExpand with OPT=ALL_V_E in GIE #2841

[GIE] Support PathExpand with OPT=ALL_V_E in GIE #2841

Conversation

BingqingLyu commented Jun 8, 2023 • edited Loading

What do these changes do?

Related issue number

codecov-commenter commented Jun 8, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BingqingLyu Jun 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

[GIE] Support `PathExpand` with `OPT=ALL_V_E` in GIE #2841

[GIE] Support `PathExpand` with `OPT=ALL_V_E` in GIE #2841

BingqingLyu commented Jun 8, 2023 •

edited

Loading

codecov-commenter commented Jun 8, 2023 •

edited

Loading

BingqingLyu Jun 12, 2023 •

edited

Loading